Support multiple instances of a module. #451

fschmenger · 2025-10-06T15:50:37Z

This PR adds a optional property moduleType to the module configuration, to make it possible to instantiate the same module multiple times. This can be useful, i.e. when you have the same or similar application logic and just require different parametrization.

To see an example, change the configuration of sample-module in app.conf to

"sample-module-inst1": {
    "moduleType": "sample-module",
    "target": "toolbar",
    "win": "floating",
    "icon": "md:star",
    "closable": false
  },
  "sample-module-inst2": {
    "moduleType": "sample-module",
    "target": "toolbar",
    "win": "floating",
    "icon": "md:star_outline",
    "closable": false
  }

To make it work, the line
~~moduleName="sample-module"~~
(which was redundant and is now generically forwarded) must be removed from SampleModule.vue:

 <wgu-module-card
    v-bind="$attrs"
    class="sample-module"
    :icon="icon"
  >

If you are using language files you must provide the title of the modules individually based on the instance name, e.g.

"sample-module-inst1": {
   "title": "Sample Module 1"
},
"sample-module-inst2": {
  "title": "Sample Module 2"
}

fschmenger · 2025-10-06T15:56:04Z

Hi guys, this draft should be easy to review (hopefully).

Let me know whether you are happy with this, then I will add a brief documentation.
Should I put a working example with 2 instances into app-conf.json? I could possible add it to sidebar (so I dont have to complicate matters to disambiguate the window position) - or do we rather keep app-conf-sidebar.json and app-conf.json in sync? Any ideas welcome...

sronveaux · 2025-11-28T09:37:09Z

Hi @fschmenger and sorry for the delay on reviewing this one. I wanted to have a time slot where I could fully think about this one and experiment a bit before replying...

At first, for sure this is quite a nice addition, easy to implement and potentially useful!
Your explanation directly gives the opportunity to test it flawlessly which is great!

Here are some thinkings/ideas/remarks that came to my mind while playing with this so we can discuss it a bit further:

In this PR, only modules which have a window assigned can be duplicated. Wouldn't the other component deserve the same functionality ? It is also a one liner to be changed in the AppHeader.vue file where type: moduleOpts.win ? 'wgu-toggle-btn' : key + '-btn', should be replaced by type: moduleOpts.win ? 'wgu-toggle-btn' : (moduleOpts.moduleType ?? key) + '-btn',. Thinking about stock Wegue components, this is not as useful but I easily could test with two Geocoder, one querying osm and the second one photon and it worked flawlessly...
Simple cosmetic which is not linked to this PR at all, if AppHeader.vue was amended as stated up here, it could be nice to change this block which has a typo:

<template v-for="(tbButton, index) in menuButtons" :key="index">
            <v-list-item class="py-0">
              <component
                  v-bind="tbButton"
                  :is="tbButton.type" :key="index"
               />
              </v-list-item>
          </template>

it should be:

<template v-for="(menuButton, index) in menuButtons" :key="index">
            <v-list-item class="py-0">
              <component
                  v-bind="menuButton"
                  :is="menuButton.type" :key="index"
               />
              </v-list-item>
          </template>

Currently, the way moduleName is defined inside components is not standardized. Some define it as a constant in their data object (like the InfoClickWin) and some define it as a constant directly inside the <wgu-module-card> property (like the LayerListWin). Wouldn't it be the opportunity to uniformize this?
Directly linked to the previous one, should we modify the stock Wegue components so they could all be instantiated multiple times? This would only mean removing the lines defining moduleName inside AttributeTableWin, HelpWin, InfoClickWin, LayerListWin, MapRecorderWin and MeasureWin (+ some unit tests). As you explained for the SampleModule, this is in all cases forwarded and so useless to define... except if we want to ensure that having multiple instances of a given component would purposely break the app.
Concerning the documentation, would it be useful to indicate that stock Wegue components were not designed to be multi-instantiated and could behave incorrectly when used in such configuration? I'm thinking about the MeasureTool for example where you can define two instances drawing with different colors for example. What to expect when you open both instances at the same time? (yes I know, it's totally useless!)
And about writing a working example, another option would be not to include it in the configuration files but add a section inside the workshop later when it will be rewritten before v3 is out... otherwise, I personally have no opinion whether app-conf-sidebar.json and app-conf.json should be kept in sync. Perhaps @chrismayer who has the whole historical view on the development could better guide us here?

Once again I wrote something way too long. The idea is just to discuss thoroughly.
To summarize with my personal opinion about all this, sure this should be implemented, I'd just add the functionality to non-win components by patching the AppHeader component and remove all the definitions of moduleName so people reading the source code for the first time would not be confused by different approaches and to enable multi-instantiation of all components. Concerning the documentation and example, I really have no strong opinion except that a full working example should be available somewhere, either in the documentation pages, either in a configuration file or either in the workshop.

Thanks again for this nice idea and addition!

…me module.

…lti instantiation.

…sts (functionality is already tested inside module core implementation).

fschmenger · 2025-12-23T16:17:07Z

Thanks for the review @sronveaux.

I rebased the branch on the current master. Everything mentioned above should be addressed now (fixing the typo should be part of a different PR).
When I return from the christmas break I can potentially add a unit test for multi-instantiation.

…nit tests to avoid warnings.

…e has been slightly altered to match WguAppTemplate.spec.js, redundant assertions have been removed.

…e stock components.

fschmenger · 2026-01-08T14:46:31Z

I finalized some missing parts in the documentation and unit tests. Happy if you can give it another review but take your time!

sronveaux

Hi @fschmenger and thanks for the effort put on this "easy to review" one 😉

I really like how you structured and wrote the documentation here, it is crystal clear and gives some details for simple instantiation too which makes this also clearer for normal usage.

Except for some suggestions, which I let you choose whether they should be applied or not, it's approved and ready to merge!

docs/module-configuration.md

tests/unit/specs/components/helpwin/HelpWin.spec.js

tests/unit/specs/components/AppHeader.spec.js

sronveaux · 2026-01-21T14:18:53Z

tests/unit/specs/WguAppTemplate.spec.js

    it('getModuleWinData(\'floating\') returns always an array', () => {
      comp = createWrapper();
      vm = comp.vm;

      // mock a window UI instance
      const moduleData = vm.getModuleWinData('floating');

      expect(moduleData).to.be.an('array');
    });

    it('getModuleWinData(\'sidebar\') returns always an array', () => {
      comp = createWrapper();
      vm = comp.vm;

      // mock a window UI instance
      const moduleData = vm.getModuleWinData('sidebar');

      expect(moduleData).to.be.an('array');
    });



Not directly related to this PR, but to align AppHeader tests and WguAppTemplate ones as stated in 20972ee perhaps those two could be removed.

I just looked into this. I think it`s too unrelated so I don't want to handle it here. Maybe we should open an issue and tackle some clean up tasks for unit tests?

Don't worry with this, I just proposed it so it would align with what you reorganized in 20972ee following the commit message:

Test structure has been slightly altered to match WguAppTemplate.spec.js, redundant assertions have been removed.

Depending on the point of view, those tests are redundant (as they are tested implicitly with the lengthof somewhere else) where some will argue unit tests should assert as few as possible and be "duplicated" to better document the code.

But I'm not sure it worths the follow-up here. Except if you see some other things to clean up in unit tests somewhere else...

tests/unit/specs/WguAppTemplate.spec.js

fschmenger · 2026-01-22T09:43:06Z

Hi @sronveaux. Thanks again for the comprehensive review.
Even though not exactly related to this PR I decided to change the 2 remaining array length assertions within the unit tests (see a7a1cc4).
I`m going to merge now.

sronveaux · 2026-01-22T09:49:51Z

Thanks @fschmenger for addressing this thoroughly and for this great feature !

Sure, the assertions were only far linked to this PR, it is always difficult to measure whether creating a separate PR for changing this worths the extra work... reason why I approved and only suggested. Thanks for taking it into account though.

All the best,
Sébastien

fschmenger requested review from chrismayer and sronveaux October 6, 2025 15:56

fschmenger marked this pull request as draft October 6, 2025 15:56

fschmenger added 5 commits December 23, 2025 15:34

Added a 'moduleType' property to support multiple instances of the sa…

a10f430

…me module.

Remove 'moduleName' data property from various components to allow mu…

a0f0c8e

…lti instantiation.

Remove 'moduleName' assertions and property declarations from unit te…

a812d01

…sts (functionality is already tested inside module core implementation).

Support multiple instances of the same toolbar button.

ac048f3

Update the documentation.

c94e592

fschmenger force-pushed the multiple_module_instances branch from 9aff4ba to c94e592 Compare December 23, 2025 16:14

fschmenger marked this pull request as ready for review December 23, 2025 16:17

fschmenger added 4 commits January 8, 2026 14:30

For module core based components declare 'moduleName' properties in u…

a110e3e

…nit tests to avoid warnings.

Add a unit test for multi module instantiation.

96f2805

Add a unit test for multi toolbar button instantiation. Test structur…

20972ee

…e has been slightly altered to match WguAppTemplate.spec.js, redundant assertions have been removed.

Add a documentation notice about multi instantiation support for Wegu…

955d09b

…e stock components.

sronveaux approved these changes Jan 21, 2026

View reviewed changes

fschmenger added 3 commits January 22, 2026 10:53

Apply proposed change to documentation.

e68ba4f

Remove empty block from unit test.

d6c4f15

Apply proposed changes to assert array lengths in various unit tests.

6b3d293

fschmenger force-pushed the multiple_module_instances branch from a7a1cc4 to 6b3d293 Compare January 22, 2026 10:05

fschmenger merged commit c28ae9c into wegue-oss:master Jan 22, 2026
1 check passed

Support multiple instances of a module. #451

Support multiple instances of a module. #451

Uh oh!

Conversation

fschmenger commented Oct 6, 2025

Uh oh!

fschmenger commented Oct 6, 2025

Uh oh!

sronveaux commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fschmenger commented Dec 23, 2025

Uh oh!

fschmenger commented Jan 8, 2026

Uh oh!

sronveaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sronveaux Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

fschmenger Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

sronveaux Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fschmenger commented Jan 22, 2026

Uh oh!

sronveaux commented Jan 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sronveaux commented Nov 28, 2025 •

edited

Loading

sronveaux Jan 22, 2026 •

edited

Loading